Skip to content

wallet: TSS external signing + TSS key storage#1018

Open
marcopeereboom wants to merge 25 commits into
mainfrom
marco_tss_external_sign
Open

wallet: TSS external signing + TSS key storage#1018
marcopeereboom wants to merge 25 commits into
mainfrom
marco_tss_external_sign

Conversation

@marcopeereboom
Copy link
Copy Markdown
Contributor

@marcopeereboom marcopeereboom commented May 11, 2026

Add external ECDSA and Schnorr signature application for TSS-signed
Bitcoin transactions, TSS key storage in zuul, and signature
verification helpers.

Depends on #971 (segwit signing PR).

TSS key type

Add TSSNamedKey to zuul and PutTSSKey/LookupKeyByAddr to zuul/memory.
TSS keys store only the public key (the private key is distributed
across TSS participants). Multi-form address indexing (P2PKH, P2WPKH,
P2TR) is shared with local keys; cross-type collisions are rejected
with distinct sentinels (ErrTSSKeyOccupied, ErrLocalKeyOccupied)
wrapping ErrKeyExists.

External signature application

TransactionApplyECDSA signs P2PKH and P2WPKH inputs given a DER-encoded
ECDSA signature and the signer's public key. TransactionApplySchnorr
signs P2TR key-path inputs given a 64-byte Schnorr signature. Both
verify pubkey-to-address binding before applying.

Verification

VerifyECDSA and VerifySchnorr validate signatures against the
transaction's sighash without requiring the private key — intended for
coordinators that receive signatures from remote TSS participants.

Tests

  • TSS end-to-end: keygen → external sign → apply → verify → script
    engine for P2PKH, P2WPKH, and P2TR inputs
  • Taproot witness crypto negative paths
  • Coverage gap tests for address mismatch, non-default sighash types
  • Cross-type key collision assertions

Blocked on hemilabs/x PR #9 (tss-lib v3 tag).

@marcopeereboom marcopeereboom requested a review from a team as a code owner May 11, 2026 11:25
@github-actions github-actions Bot added area: docs This is a change to documentation changelog: done This pull request includes an appropriate update to CHANGELOG.md. labels May 11, 2026
@socket-security
Copy link
Copy Markdown

socket-security Bot commented May 11, 2026

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedgolang/​github.com/​hemilabs/​x/​tss-lib/​v3@​v3.0.0-20260507172513-c23bec7119b999100100100100

View full report

@codecov
Copy link
Copy Markdown

codecov Bot commented May 11, 2026

@marcopeereboom marcopeereboom force-pushed the marco_tss_external_sign branch 4 times, most recently from ab0d6f3 to 0eb787f Compare May 12, 2026 08:00
@github-actions github-actions Bot added the area: tbc This is a change to TBC (Tiny Bitcoin) label May 12, 2026
A key stored via PutKey is now simultaneously indexed under the
P2PKH, P2WPKH, and BIP-86 P2TR addresses that derive from its
public key.  LookupKeyByAddr and GetKey succeed for any of the
three forms without requiring the caller to know which address
type was used at enrollment.

PurgeKey recomputes all three addresses from the stored public
key and removes every entry before zeroing the private key, so
purging via any address form fully evicts the key.

The PutKey all-or-nothing check fails with ErrKeyExists if any
of the target addresses already points at a stored key,
preventing partial inserts on collision.
Change the prevOuts return type from map[string][]byte to
PrevOuts (map[string]*wire.TxOut) so the previous output's
amount travels alongside its pkScript.

BIP-143 (segwit v0) and BIP-341 (taproot) sighash algorithms
commit to the spent amount.  Without the amount available at
signing time, TransactionSign cannot support witness-based
inputs.

TransactionCreate and PoPTransactionCreate now emit the richer
structure; TransactionSign consumes it.  Internal callers
(service/popm) treat prevOuts as opaque and need no changes.
No signing behaviour changes; this commit only makes the
amount available for subsequent signing-path additions.
TransactionSign now dispatches per input based on the previous
output's script class.  Inputs with a WitnessV0PubKeyHashTy
pkScript are signed through txscript.WitnessSignature using the
BIP-143 sighash; all other classes continue to flow through
txscript.SignTxOutput and produce a SignatureScript.

The BIP-143 sighash midstate (TxSigHashes) is computed once per
transaction via NewTxSigHashes and shared across witness inputs,
matching standard practice for multi-witness transactions.

prevOutsFetcher adapts PrevOuts to txscript.PrevOutputFetcher.
signP2WPKH extracts the native segwit address from the witness
program, looks up the key in zuul (which indexes keys under
both legacy and segwit forms), and emits the two-element
witness stack (sig, pubkey).

The existing PoP signing path (TestIntegration) is unchanged in
behaviour and continues to pass: P2PKH inputs take the default
branch and reach SignTxOutput exactly as before.
PoP transactions are critical-path and must remain functional as
signing paths are added.  These tests lock down the current PoP
flow against future regressions.

TestPoPTransactionStructure verifies PoPTransactionCreate emits
exactly one input, a zero-value OP_RETURN output carrying the
abbreviated keystone, and a PrevOuts entry holding both value
and pkScript.  The OP_RETURN round-trips through
pop.ParseTransactionL2FromOpReturn back to the same keystone.

TestPoPTransactionSignValidates signs a PoP tx and runs the
script engine against the signed input, proving the legacy
P2PKH dispatch in TransactionSign still produces valid
signatures after the script-class refactor.

TestPoPNoWitnessDataLeak asserts a signed P2PKH PoP tx has no
witness data on any input.  Witness data on a legacy input is a
protocol violation that some nodes reject.

TestPoPSighashCacheSafeOnLegacyOnly is a regression guard for
the unconditional NewTxSigHashes call introduced in the P2WPKH
work.  For legacy-only transactions that call must not panic,
must not corrupt the fetcher, and must leave the P2PKH path
correct across repeated iterations.

TestPoPPrevOutsFetcherRoundTrip exercises prevOutsFetcher
directly against a PoP-shaped prevOuts map.  A silent key-parse
failure in the adapter would cause NewTxSigHashes to dereference
a nil TxOut and panic; this test asserts the fetcher returns
the expected amount and pkScript for the PoP funding input.
TransactionSign now handles WitnessV1TaprootTy inputs via BIP-86
key-path spends.  signP2TRKeyPath looks up the untweaked internal
key in zuul (indexed under the BIP-86 P2TR address) and calls
txscript.RawTxInTaprootSignature with a nil script root.

The helper must pass the untweaked key because
RawTxInTaprootSignature applies the BIP-341 tweak internally.
Double-tweaking produces a signature for the wrong output key
that the script engine rejects with ErrTaprootSigInvalid.

Script-path taproot spends (tapscript with a committed envelope
or other leaf script) are out of scope for this entry point.
They require the caller to supply the leaf script and control
block, which the wallet does not carry.  Such inputs must be
signed by a dedicated entry point before calling TransactionSign.

verifyInput in the test suite now takes the full PrevOuts map
instead of a single TxOut.  Taproot sighash commits to every
input's pkScript and amount, so a single-input fetcher produces
a sighash mismatch on mixed-input transactions.  All test
call sites updated.

PoP regression tests pass: legacy P2PKH inputs still flow through
the default SignTxOutput branch with no witness data, and the
sighash cache computation remains safe for legacy-only
transactions.
The Gozer interface now exposes TxByID(ctx, txid) returning
*tbcapi.Tx, filling in the obvious sibling to the existing
BroadcastTx — both are transaction-oriented, and tbcd already
exposes the backing TxByIdRequest / TxByIdResponse pair.

Prior to this change any Gozer consumer that needed to inspect
an on-chain transaction (for example to decode the tapscript
witness of an Ordinals reveal) had to either reach outside the
abstraction to a block explorer REST API, or open its own
WebSocket against tbcd for a single call. Neither is acceptable
for a library whose whole job is to be the wallet's source of
chain data. TxByID closes the gap.

tbcgozer implementation mirrors BroadcastTx line for line:
build a tbcapi.TxByIdRequest, callTBC, type-assert the response,
surface the protocol.Error if any, otherwise return the Tx.
Nil txid returns a clean error instead of panicking on the
dereference inside the request struct.

blockstream implementation is a "not supported yet" stub,
consistent with BlocksByL2AbrevHashes and KeystonesByHeight.
Blockstream exposes GET /tx/{txid} and GET /tx/{txid}/hex which
could be mapped onto *tbcapi.Tx, but no current consumer uses
blockstream for tx introspection — fill in when that changes.

Adding a method to Gozer is a breaking change for any external
implementer of the interface. There are none in this repo and
none known externally; tbcGozer and blockstreamGozer are the
only concrete types.

No new dependencies. Existing bitcoin/wallet/... test suite
passes: wallet, blockstream, tbcgozer, vinzclortho, zuul/memory.
…tests

Add CmdTxByIdRequest handler to mock TBC server so tbcGozer.TxByID
can be exercised without a live TBC instance.

tbcgozer: three new tests cover nil txid, not-connected, and happy
path (including concurrent queue-depth exercise).

blockstream: confirm stub returns not-supported-yet.

wallet: extend TestIntegration to call TxByID after BroadcastTx and
verify the returned transaction fields.
mock: zero hash now triggers error response for TxByIdRequest,
enabling callers to exercise the resp.Error path.

tbcgozer: two new negative tests cover TBC error response
propagation (zero hash) and context cancellation before RPC.

tbcgozer: FuzzTBCGozerTxByID exercises TxByID with arbitrary
txid bytes through the mock to catch marshaling edge cases.
Add targeted tests for error-return branches reachable from the
public API:

- UtxoPickerMultiple / UtxoPickerSingle no-match and skip-too-small
  paths.
- TransactionSign error-wrap paths for unknown P2WPKH and P2TR
  keys, confirming the per-class dispatch propagates resolveInput-
  SigningKey failures with input index and class in the wrapping.
- TransactionSign pre-validation: missing PrevOuts entry returns a
  clean error naming the offending input instead of panicking in
  witness sighash midstate computation.
- prevOutsFetcher defensive panic on a malformed outpoint key.
signP2WPKH and signP2TRKeyPath set the witness but never cleared
SignatureScript.  TransactionCreate pre-populates SignatureScript
with the pkScript for all inputs.  For native segwit (P2WPKH, P2TR),
scriptSig must be empty — a non-empty scriptSig causes the script
engine to reject the transaction with a clean-stack violation.

Add regression tests that pre-populate SignatureScript the way
TransactionCreate does and verify it is cleared after signing.
Drop underscore digit separators from numeric literals to match
codebase convention.  Unwrap multiline PutKey if-init statements
into separate assignment and error check.
Add unit tests for three tbc fixes that landed without coverage:

CPFP mempool resolution (parseTx fallback):
- TestTxOutByOutpoint: parent output resolved from mempool
- TestTxOutByOutpointNotFound: missing txid returns nil
- TestTxOutByOutpointBadIndex: out-of-range index returns nil
- TestParseTxCPFP: child tx resolves input value from unconfirmed
  parent in mempool when block database has no record
- TestParseTxCPFPNoMempool: fails when no mempool provided
- TestParseTxCPFPParentNotInMempool: fails when parent absent from
  both database and mempool

MaxResponseSize:
- TestMaxResponseSize: assert the websocket read limit constant is
  16 MiB, large enough for worst-case block hex payloads
TSSNamedKey represents a key controlled by an external threshold
signature scheme.  No private material is held locally; the struct
carries only the aggregated group public key and an opaque keyID
that external signers use to identify which distributed key to
sign with.

The Zuul interface grows four symmetrical methods: PutTSSKey,
GetTSSKey, PurgeTSSKey, LookupTSSKeyByAddr.  These parallel the
existing local-key methods but dispatch to a separate in-memory
index so the two key types share address namespace without
overlapping.

TSS keys are indexed under P2PKH and P2WPKH only.  Taproot P2TR
key-path spends require schnorr signatures; ECDSA TSS (the
variant this branch initially integrates) cannot satisfy a
BIP-341 key-path spend, so exposing a TSS key under a P2TR
address would be a footgun for callers trying to build a send
from that address.  Schnorr TSS, when added later, will get its
own enrolment path that includes P2TR.

Collisions are detected bidirectionally: enrolling a local key
at an address already held by a TSS key (and vice versa) returns
ErrKeyExists.  PurgeTSSKey removes every indexed address form
in a single call, mirroring the multi-address behaviour of the
local PurgeKey.

Tests cover: P2PKH+P2WPKH indexing with P2TR explicitly excluded;
purge round-trip from any address form; field validation
(nil struct, missing PublicKey, zero-length KeyID); collision
detection in both directions between PutKey and PutTSSKey.

Existing PoP and signing tests pass unchanged.
TransactionApplyECDSA wires a pre-computed DER-encoded ECDSA
signature into a specific transaction input.  The signature is
produced out of band — by a hardware wallet, a PSBT flow, or a
threshold signature committee — and the wallet only handles the
witness or sigScript assembly.

P2PKH inputs receive the standard two-push SignatureScript
(<sig||hashType> <pubKey>).  P2WPKH inputs receive a two-element
witness stack ([sig||hashType, pubKey]).  Other script classes
are rejected with an explicit error: P2TR requires schnorr, and
wrapped-segwit/P2WSH variants are not yet supported.

Before applying, the function cross-checks the provided pubkey
against the address embedded in the prev pkScript.  A mismatch
would produce a transaction the network rejects on broadcast;
catching it at injection time surfaces the bug at the caller.

ECDSASigFromRS assembles a DER signature from raw big-endian r
and s scalar bytes, the shape commonly emitted by ECDSA TSS
signing libraries.  The helper rejects empty scalars, zero
scalars, and scalars that overflow the secp256k1 group order.
Low-S normalisation (BIP-146) is performed implicitly by
Signature.Serialize, so high-S TSS output is accepted by the
helper and emitted as low-S.

Tests exercise: round-trip verify of assembled DER signatures;
rejection of bad scalars; low-S normalisation of a high-S
input; end-to-end P2PKH injection with script-engine
verification; end-to-end P2WPKH injection with BIP-143 sighash
and engine verification; wrong-pubkey rejection; and
unsupported-script-class (P2TR) rejection.

PoP and existing signing tests pass unchanged.
TransactionApplySchnorr wires a pre-computed 64-byte BIP-340
schnorr signature into a P2TR key-path input.  The sibling of
TransactionApplyECDSA, this is the injection path for schnorr
threshold signature schemes (MuSig2, FROST, schnorr-TSS) that
produce aggregated signatures the wallet cannot sign locally.

pubKey is the untweaked internal key; the function applies the
BIP-86 tweak via ComputeTaprootKeyNoScript and cross-checks the
tweaked x-only output key against the witness program in the
prev pkScript.  A mismatch is reported before the transaction
is mutated.

For SigHashDefault (the common case) the witness stack is the
bare 64-byte signature per BIP-341.  Any other sighash type
appends the single sighash byte.

schnorr.ParseSignature is called on the input sig as a cheap
structural check: it validates the 64-byte length and catches
grossly malformed encodings.  On-curve validity of R.x is only
checked during actual Verify, so callers wanting a real
cryptographic check before broadcast should use VerifySchnorr.

Only BIP-86 key-path spends are supported.  Script-path taproot
spends require a committed leaf script and a control block;
those inputs must be assembled by the caller.

Tests cover: round-trip sign + inject + engine verification on
a real P2TR input; structural rejection (wrong length, nil
pubkey, empty signature); wrong-key rejection via tweak
mismatch; and unsupported-script-class (P2PKH) rejection.

PoP regression tests continue to pass.
VerifyECDSA and VerifySchnorr are pre-broadcast sanity helpers
for externally-computed signatures.  Callers producing a
signature out of band (TSS committee, hardware wallet, PSBT
flow) can run the result through these helpers before handing
it to TransactionApplyECDSA or TransactionApplySchnorr to
catch wrong-key or wrong-hash errors at injection time rather
than on broadcast.

VerifyECDSA parses a DER-encoded signature (no trailing sighash
byte) and checks it against a 32-byte sighash under the
provided public key.  Structural guards reject nil pubkey,
wrong-length sighash, empty signature, and malformed DER.

VerifySchnorr is the BIP-340 counterpart.  The caller supplies
the tweaked x-only output key (not the internal key), the
32-byte BIP-341 sighash, and the 64-byte schnorr signature.

Tests cover: happy-path verification for both algorithms;
wrong-key rejection; wrong-hash rejection for ECDSA; structural
rejection of nil/short/malformed inputs; and a taproot
round-trip exercising the tweak flow a real TSS caller would
follow.

PoP regression tests continue to pass.
TestTSS_E2E_P2WPKH proves that an externally-produced ECDSA
threshold signature can be injected into a bitcoin transaction and
accepted by the same script engine a bitcoin node runs against
witnessed inputs.

The test runs a real 2-of-3 ECDSA TSS ceremony in-process using
github.com/hemilabs/x/tss-lib/v3: full Paillier pre-parameter
generation, 4-round distributed keygen, 9-round distributed
signing + finalize.  The private key exists only as shares across
the committee at every moment of the test.  No mocks, no
single-party shortcuts.

The group public key is turned into a P2WPKH testnet address, an
unsigned spend is built against a funding UTXO locked to that
address, the BIP-143 sighash is computed, and the committee signs
the sighash.  The resulting raw (r, s) scalars flow through
ECDSASigFromRS, VerifyECDSA, and TransactionApplyECDSA, and the
final transaction is handed to txscript.NewEngine for consensus
validation.

Gated behind the tss_e2e build tag.  Paillier safe-prime
generation takes roughly 50 seconds for a 3-party run, and the
full test finishes in about one minute.  Regular make test does
not build this file, so the default test suite remains fast.

Run locally with:

    go test -tags tss_e2e -timeout 15m \
        -run TestTSS_E2E ./bitcoin/wallet/

Adds github.com/hemilabs/x/tss-lib/v3 as a direct test-only
dependency pinned to the max/tss_changes branch tip.  Will follow
the upstream tss-lib v3 release once that repo tags a version.
Record the wallet changes introduced by this branch under the
Unreleased section: external ECDSA/schnorr signature injection
(TransactionApplyECDSA, TransactionApplySchnorr, ECDSASigFromRS,
VerifyECDSA, VerifySchnorr), native P2WPKH and BIP-86 P2TR
key-path signing in TransactionSign, and the TSSNamedKey storage
type with its PutTSSKey/GetTSSKey/PurgeTSSKey/LookupTSSKeyByAddr
interface on zuul.

The prevOuts return-type change on TransactionCreate and
PoPTransactionCreate is recorded under Changed as a minor
breaking change for external consumers; internal callers
(service/popm) treat prevOuts opaquely and are unaffected.
Codecov reported 77.88% patch coverage with 69 uncovered lines on
the branch.  The vast majority were error-return branches
reachable from the public API but not exercised by the existing
test suite.  Add targeted tests to close the callable gaps:

- UtxoPickerMultiple / UtxoPickerSingle no-match and skip-too-small
  paths.
- TransactionSign error-wrap paths for unknown P2WPKH and P2TR
  keys, confirming the per-class dispatch propagates resolveInput-
  SigningKey failures with input index and class in the wrapping.
- prevOutsFetcher defensive panic on a malformed outpoint key,
  asserted via recover().  Without this guard, a caller-crafted
  bad key would silently produce a corrupt sighash midstate.
- TransactionApplyECDSA address-mismatch rejection on the P2WPKH
  branch of pubKeyMatchesAddress, the sibling to the existing
  P2PKH-branch coverage.
- TransactionApplySchnorr witness assembly for non-SigHashDefault
  sighash types, verifying the trailing sighash byte is appended
  per BIP-341.

Lifts statement coverage from 85.8% to 90.8% in bitcoin/wallet;
patch coverage on the branch rises accordingly.  The remaining
uncovered branches are defensive wraps around btcd library calls
that cannot fail on well-formed inputs (address derivation from
valid pubkey bytes, SignatureScript building, etc.) and are not
reachable without mocking dependencies Tobias forbids mocking.
Existing tests cover happy-path signing and verification for the
BIP-86 key-path P2TR signer, the external schnorr injection
helper, and VerifySchnorr.  This adds the adversarial and
structural cases that prove the primitives actually enforce the
BIP-341 commitments they claim to:

  - tampered-tx: signed tx survives mutation of output value,
    output script, input sequence, or prev amount.  Each
    mutation must invalidate the witness.  The prev-amount case
    is distinctly taproot — BIP-341 commits to every input's
    prev-amount, segwit v0 does not.
  - wrong-key: spend a UTXO whose pkScript commits to key B
    using only key A in zuul.  Must fail with the
    lookup-key-does-not-exist error rather than silently
    signing.
  - malformed pkScript at the resolve-input-signing-key layer:
    truncated push opcodes and OP_RETURN.  Both must error out
    before any signing attempt.
  - pubKeyMatchesTaprootAddress: rejects the wrong key, rejects
    an untweaked internal key when the pkScript commits to a
    raw pubkey (demonstrates the helper applies BIP-86 tweak
    correctly), rejects malformed pkScripts.
  - VerifySchnorr parse failures: a 64-byte signature whose r
    is the field prime (out of range) forces
    schnorr.ParseSignature to error; a 32-byte pubkey with
    x = field prime forces schnorr.ParsePubKey to error.  These
    exercise the two parse branches missed by structural
    (length-based) negative tests.
  - schnorr external-sign roundtrip: compute BIP-341 sighash,
    sign externally with the tweaked key, VerifySchnorr against
    the tweaked x-only output key, TransactionApplySchnorr,
    verify via script engine.  Plus bit-flip negative control.
  - cross-input replay: two txs spending different outpoints
    under the same pkScript.  Swapping the witness between them
    must fail — sighash commits to the outpoint.

Coverage lift in bitcoin/wallet:

  before: 91.2%   after: 92.9%

  VerifySchnorr              86.7% -> 100.0%
  pubKeyMatchesTaprootAddress 75.0% -> 83.3%
  resolveInputSigningKey      72.7% -> 81.8%

The remaining gap is unreachable without mocking btcd:
ExtractPkScriptAddrs never returns a non-nil error in
btcd v0.24.3 (every branch returns nil), and
btcutil.NewAddressTaproot only rejects non-32-byte input which
schnorr.SerializePubKey never produces.  The defensive
error-returning code stays for forward compatibility if btcd
tightens its parsers.
PutKey now returns ErrTSSKeyOccupied when a TSS key blocks the
address, and PutTSSKey returns ErrLocalKeyOccupied when a local
key blocks it.  Both wrap ErrKeyExists for backward compatibility.

Update TestPutKeyVsPutTSSKeyCollision to assert the specific
sentinel while also verifying the ErrKeyExists wrapping.
Improve error messages: use errors.New where no format verbs, clarify
nil-argument and verify-failure wording. Remove underscore separators
from number literals in tests. Replace append-copy idiom with
preallocate+copy for sigWithHash and schnorr witness construction.
…re-encoding

Round-trip ECDSA and schnorr signatures through parse+serialize to
guarantee canonical encoding. Remove redundant length pre-checks
and maxECDSASigDERLen constant since the parsers validate length
internally.
Caught by golangci-lint --fix; missed in prior review cleanup.
@marcopeereboom marcopeereboom force-pushed the marco_tss_external_sign branch from 0eb787f to 975df46 Compare May 12, 2026 08:06
@joshuasing joshuasing added the status: blocked This is blocked by something else label May 12, 2026
@joshuasing joshuasing added this to the v2.1.0 milestone May 19, 2026
Comment thread go.mod
github.com/ethereum/go-ethereum v1.17.2
github.com/go-test/deep v1.1.1
github.com/golang-jwt/jwt/v5 v5.3.1
github.com/hemilabs/x/tss-lib/v3 v3.0.0-20260507172513-c23bec7119b9
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must be changed to use github.com/hemilabs/x/tss/v3 once merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: docs This is a change to documentation area: tbc This is a change to TBC (Tiny Bitcoin) changelog: done This pull request includes an appropriate update to CHANGELOG.md. status: blocked This is blocked by something else

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants